feat(github-app): connect-existing flow, revoke on delete, ownership verification#45
Merged
Merged
Conversation
added 3 commits
May 13, 2026 23:21
…verification
Addresses the "stuck after workspace delete" UX bug along with several
audit findings surfaced while mapping the install/setup/webhook surface:
the setup callback was vulnerable to installation_id parameter spoofing,
workspace delete silently leaked active Polar/Stripe subscriptions, and
`installation.suspend`/`unsuspend` webhook events were dropped.
Patterns adopted from PostHog (gold-standard reference) and the legacy
Contentrain CMS connect-existing UX, both inspected end-to-end before
designing the fix.
Highlights:
- New `oauth_provider_tokens` table — encrypted GitHub user OAuth
tokens (`gho_*`/`ghu_*`) captured at sign-in via the new
`ProviderTokens` shape on `AuthSession`. Required because Supabase
Auth surfaces these only on the immediate callback and never
persists them server-side. AES-256-GCM via the existing
`server/utils/encryption.ts` helper (rotation-aware via
NUXT_SESSION_SECRET_PREVIOUS).
- New `AuthProvider.refreshProviderToken('github', refreshToken)` —
calls `POST https://github.com/login/oauth/access_token` directly
(Supabase only refreshes its own JWT). Used by the lazy refresh
helper `server/utils/github-token.ts:getValidGitHubUserToken`
which transparently rotates the 8h-TTL user-to-server token
(PostHog pattern) and emits `GITHUB_REAUTH_REQUIRED` to drive the
Reconnect-GitHub UI when the refresh token has expired or been
revoked.
- New `GitAppService` provider (`useGitAppService()`) — App-level
GitHub operations not scoped to a single installation:
`listInstallationsForUser` (user OAuth token, filtered to the
Studio App ID) and `verifyUserHasAccessToInstallation` (PostHog-
derived ownership check via `GET /user/installations/{id}/repositories`).
- `GitAppProvider.revokeInstallation()` (instance method) — App-JWT
`DELETE /app/installations/{id}` call, idempotent on 404.
- New endpoints
`GET /api/github/installations/available` (annotated with bound
status) and `POST /api/github/installations/connect`
(ownership-verified attach for the "already installed → Configure"
recovery flow that GitHub's `installations/new` URL cannot trigger
a callback for).
- `setup.get.ts` now verifies the caller actually has GitHub-side
access to the installation_id they supplied — closes the
application-trust invariant gap audited in `findWorkspaceByGithubInstallation`
(the previous 409 collision check was the only protection against
arbitrary cross-tenant attach attempts).
- `DELETE /api/workspaces/[id]` accepts two opt-in body flags:
`uninstallGithubApp` (revoke when no other workspace shares the
installation) and `cancelSubscription` (call `payment.cancelSubscription`
before CASCADE drops the payment_accounts row). Both best-effort —
failure logged, not blocking. Matches the audit finding that none
of the surveyed OSS projects (PostHog, Coolify, OpenSauced) revoke
on tenant delete, treating it as opt-in operator action.
- Webhook handler now responds to `installation.suspend` (status →
'suspended'), `installation.unsuspend` (→ 'active'), and
`installation.created` (defensive status confirmation), in
addition to the existing `installation.deleted` handling. The new
`workspaces.github_installation_status` column lets the UI
distinguish 'active' / 'suspended' / 'unbound' instead of inferring
from `github_installation_id` alone.
- `ConnectRepoDialog` install state offers "Or connect an existing
installation" — a picker that lists the user's GitHub-visible
installations (with already-bound items disabled) and reaches the
`/connect` endpoint. Surfaces a Reconnect-GitHub empty state on
`GITHUB_REAUTH_REQUIRED`.
- Workspace delete confirmation dialog gains two opt-in switches via
a new `extra` slot on `ConfirmDeleteDialog`. The slot is generic
— existing callers (project delete, AI keys, etc.) are unchanged.
Files touched: 20 modified, 5 new (including `004_github_installation_lifecycle.sql`).
Test coverage: 11 new integration cases on top of 567 pre-existing
(578 total, all passing). Lint clean (0 errors).
…andling)
Closes the second half of the GitHub App lifecycle picture surfaced
during review. Workspace-level installation lifecycle was already
handled in the previous commit (suspend/unsuspend/uninstall on the App
boundary). This commit adds project-level repo access tracking —
needed because a user can leave the App installed but revoke access
to one specific repo (manual unselect in App settings, org admin
action, or repo deletion on GitHub's side). In those cases the
installation token is still valid but every commit/branch call for
that project would 404 with no actionable UI feedback.
DB:
- New migration `005_project_access_status.sql` adds `projects.access_status`
('accessible' | 'inaccessible' | 'deleted', default 'accessible')
and an index on `projects.repo_full_name` for webhook lookups. The
new column is orthogonal to the existing `status` column (which
tracks setup state: active/setup/error) — a project can be
`status='active'` AND `access_status='inaccessible'`.
Webhook handlers (`server/api/webhooks/github.post.ts`):
- `installation_repositories.added` — restore matching projects to
`accessible` when the App regains access to a previously-revoked
repo (e.g. user re-selects it in the App settings).
- `installation_repositories.removed` — flip matching projects to
`inaccessible`.
- `repository.deleted` — flip to `deleted` (terminal state; project
becomes read-only and the only recovery is disconnect).
- `repository.renamed` — update `repo_full_name` so subsequent API
calls hit the new path. The old name comes from
`changes.repository.name.from`; we prepend the owner login from
`repository.owner.login` to reconstruct the full old name.
All four handlers are scoped to the matching `installation_id` to
prevent cross-tenant collisions on the same `repo_full_name`.
DB methods:
- `updateProjectAccessStatus({installationId, repoFullName}, status)` —
resolves `workspaces.id` set for the installation, then UPDATEs
matching projects. Two-step because supabase-js doesn't expose
JOIN-update via its builder.
- `renameProjectRepo({installationId, oldFullName}, newFullName)` —
same shape for the rename case.
UI:
- `AppSidebar.vue` project links render a small status dot when
`access_status !== 'accessible'` (warning-400 for inaccessible,
danger-400 for deleted) — matches the existing healthScore badge
pattern. Tooltip carries the per-state label.
- Project page (`pages/w/[slug]/projects/[projectId]/index.vue`)
shows a banner above the chat/content panels with:
- icon + title + hint per status
- "Manage in GitHub" CTA linking to the App's settings page
(omitted in the `deleted` case — nothing to manage there)
- New i18n strings: github.repo_access_revoked_{title,hint},
github.repo_deleted_{title,hint}, github.manage_app_settings_button,
projects.access_inaccessible_badge, projects.access_deleted_badge.
Tests:
- 4 new integration cases on `github-webhook.integration.test.ts`:
installation_repositories.added/removed, repository.deleted, and
repository.renamed. Full suite: 582/582 (was 578).
Files: 8 modified, 1 new (337+/1-).
Pulls in PR #44 (fix(billing): redirect free workspace to checkout). Auto-merge clean — both branches touched ConnectRepoDialog.vue but in non-overlapping regions: this branch added the connect-existing state machine, main's commit added 402 + requiresCheckout handling in the catch block. Both retained.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Solves the "stuck after workspace delete" UX bug end-to-end, plus the audit findings surfaced while mapping the install/setup/webhook/delete surface.
installations/newpage short-circuits to "Configure" when the App is already installed, and Configure doesn't fire a callback (GitHub Community #42351) — so the user couldn't bind the existing installation to a new workspace without manually uninstalling from GitHub UI first.What changed
Backend (provider layer + endpoints)
oauth_provider_tokenstable — encrypted GitHub user OAuth tokens (gho_*/ghu_*) captured at sign-in. AES-256-GCM via existingserver/utils/encryption.ts(rotation-aware).AuthProvider.refreshProviderToken('github', refreshToken)— calls GitHub's OAuth refresh endpoint directly (Supabase only refreshes its own JWT). Used by lazy refresh helperserver/utils/github-token.ts:getValidGitHubUserToken(PostHog pattern, 8h TTL handling).GitAppService(factoryuseGitAppService()) — App-level operations:listInstallationsForUserandverifyUserHasAccessToInstallation(PostHog-derived ownership check viaGET /user/installations/{id}/repositories).GitAppProvider.revokeInstallation()— App-JWTDELETE /app/installations/{id}, idempotent on 404.GET /api/github/installations/available(with bound annotation) andPOST /api/github/installations/connect(ownership-verified attach).setup.get.tsnow ownership-verifies the suppliedinstallation_id— closes the installation_id parameter spoofing gap found in audit.DELETE /api/workspaces/[id]accepts opt-in body flagsuninstallGithubAppandcancelSubscription, both best-effort.installation.suspend/unsuspend/createdin addition todeleted. Newworkspaces.github_installation_statuscolumn tracks 'active' / 'suspended' / 'unbound'.Frontend
ConnectRepoDialoginstall state offers "Or connect an existing installation" — picker shows user's GitHub-visible installations, bound items disabled. Surfaces "Reconnect GitHub" empty state onGITHUB_REAUTH_REQUIRED.extraslot onConfirmDeleteDialog(slot is generic — existing callers unchanged).Patterns referenced
posthog/models/github_integration_base.py:133-164), user-OAuth token refresh implementation (posthog/models/user_integration.py:187-222), separate install / link-existing endpoints (posthog/api/integration.py:1055-1162)./Users/.../old_contentrain) —GET /github/authorized-user-installations/:github_idlisting + popup polling pattern + per-(user, provider) token row.Test plan
pnpm test— 578 / 578 pass (11 new integration cases)pnpm lint— 0 errors (pre-existing warnings only)pnpm typecheck— clean004_github_installation_lifecycle.sqloauth_provider_tokensrow createduninstallGithubAppflag — GitHub App stays, can be reattached via "Connect existing" → no need to uninstall manuallyuninstallGithubAppflag — App revoked, next install flow starts freshcancelSubscriptionflag — Polar subscription cancelled (check Polar dashboard)installation.suspendin GitHub UI — workspace status flips to 'suspended'installation_idthe user doesn't own (manual API call) — rejected 403Files
supabase/migrations/004_github_installation_lifecycle.sqlserver/api/github/installations/server/utils/github-token.ts,server/providers/supabase-db/oauth-tokens.tstests/integration/github-installations.integration.test.tsOut-of-scope follow-ups (audit findings deferred)
workspaces.github_installation_id+ advisory lock aroundsetup.get.tsfind/update — separate hardening PR (low priority — application-level 409 check + ownership verification already cover most of this).incomplete_expiredstate mapping — billing edge case unrelated to this lifecycle work.server/api/workspaces/index.post.ts:13-14referencing old Stripe trial flow.